Skip to content

gh-132558: Improve argparse docs on combining type and choices #133827

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 8 commits into from
Sep 17, 2025
Merged

gh-132558: Improve argparse docs on combining type and choices #133827

merged 8 commits into from
Sep 17, 2025

Conversation

hansthen
Copy link
Contributor

@hansthen hansthen commented May 10, 2025 •
edited by github-actions bot
Loading

This clarifies the documentation a bit to warn users of the potential side effects of using both type and choices on an action. It also suggests that it is better in those cases to do type conversion in application code.

📚 Documentation preview 📚: https://cpython-previews--133827.org.readthedocs.build/

@bedevere-app bedevere-app bot added docs Documentation in the Doc dir skip news labels May 10, 2025
@bedevere-app bedevere-app bot mentioned this pull request May 10, 2025
Open
@github-project-automation github-project-automation bot moved this to Todo in Docs PRs May 10, 2025
@picnixz picnixz changed the title gh-132558: Improve docs on combining *type* and *choices* gh-132558: Improve argparse docs on combining type and choices May 10, 2025
StanFromIreland
StanFromIreland reviewed May 10, 2025
View reviewed changes
Doc/library/argparse.rst
Comment on lines 1130 to 1133
Formatted choices override the default *metavar* which is normally derived
from *dest*. This is usually what you want because the user never sees the
*dest* parameter. If this display isn't desirable (perhaps because there are
many choices), just specify an explicit metavar_.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be inbetween the choices notes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was already with the choices documentation. Where would you move it to?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should move the text you added above it.

Doc/library/argparse.rst Outdated
Comment on lines 1141 to 1142
Use of :class:`enum.Enum` is not recommended because it is difficult to
control its appearance in these messages.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's previous place was fine, it continued naturally from the previous point.

savannahostrowski
savannahostrowski reviewed May 19, 2025
View reviewed changes
savannahostrowski
savannahostrowski requested changes Jul 8, 2025
View reviewed changes
Copy link
Member

@savannahostrowski savannahostrowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just adding a new line back but otherwise, I think this looks good to go.

@bedevere-app
Copy link

bedevere-app bot commented Jul 8, 2025

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!

@savannahostrowski savannahostrowski added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Jul 8, 2025
savannahostrowski
savannahostrowski approved these changes Sep 17, 2025
View reviewed changes
Copy link
Member

@savannahostrowski savannahostrowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@savannahostrowski savannahostrowski enabled auto-merge (squash) September 17, 2025 15:19
@savannahostrowski savannahostrowski merged commit dd0840b into python:main Sep 17, 2025
30 of 31 checks passed
@github-project-automation github-project-automation bot moved this from Todo to Done in Docs PRs Sep 17, 2025
@miss-islington-app
Copy link

Thanks @hansthen for the PR, and @savannahostrowski for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 17, 2025
@hansthen @savannahostrowski @miss-islington
pythongh-132558: Improve argparse docs on combining type and `cho…
799927e 
…ices` (pythonGH-133827)

(cherry picked from commit dd0840b)

Co-authored-by: Hans Then <[email protected]>
Co-authored-by: Savannah Bailey <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 17, 2025
@hansthen @savannahostrowski @miss-islington
pythongh-132558: Improve argparse docs on combining type and `cho…
bdc9c55 
…ices` (pythonGH-133827)

(cherry picked from commit dd0840b)

Co-authored-by: Hans Then <[email protected]>
Co-authored-by: Savannah Bailey <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Sep 17, 2025

GH-139057 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Sep 17, 2025
@bedevere-app
Copy link

bedevere-app bot commented Sep 17, 2025

GH-139058 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Sep 17, 2025
savannahostrowski added a commit that referenced this pull request Sep 17, 2025
@miss-islington @hansthen @savannahostrowski
[3.13] gh-132558: Improve argparse docs on combining type and `ch…
216f655 
…oices` (GH-133827) (#139058)

gh-132558: Improve `argparse` docs on combining `type` and `choices` (GH-133827)
(cherry picked from commit dd0840b)

Co-authored-by: Hans Then <[email protected]>
Co-authored-by: Savannah Bailey <[email protected]>
hugovk pushed a commit that referenced this pull request Sep 17, 2025
@miss-islington @hansthen @savannahostrowski
[3.14] gh-132558: Improve argparse docs on combining type and `ch…
7a2854e 
…oices` (GH-133827) (#139057)

gh-132558: Improve `argparse` docs on combining `type` and `choices` (GH-133827)
(cherry picked from commit dd0840b)

Co-authored-by: Hans Then <[email protected]>
Co-authored-by: Savannah Bailey <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@StanFromIreland StanFromIreland StanFromIreland left review comments

@savannahostrowski savannahostrowski savannahostrowski approved these changes

Assignees
No one assigned
Labels
docs Documentation in the Doc dir skip news
Projects
Docs PRs
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hansthen @savannahostrowski @StanFromIreland